Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use same base image as thick plugin #1298

Conversation

adrianchiris
Copy link
Contributor

@adrianchiris adrianchiris commented Jun 16, 2024

change thin plugin dockerfile to use the same
base image as the thick plugin.

this will allow users to implement simple lifecycle hooks e.g exec simple shell cmd to delete multus conf file on container exit which allows to remove multus (thin) from the cluster without breaking it.

this functionality was possible in v3.

Related issue: #592

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
change thin plugin dockerfile to use the same
base image as the thick plugin.

this will allow users to implement simple lifecycle
hooks e.g exec simple shell cmd to delete multus conf
file on container exit which allows to remove multus
from the cluster without breaking it.

this functionality was possible in v3.

Related issue: k8snetworkplumbingwg#592

Signed-off-by: adrianc <adrianc@nvidia.com>
@coveralls
Copy link

Coverage Status

coverage: 63.364% (+0.2%) from 63.116%
when pulling 4e1bca1 on adrianchiris:use-same-base-image
into aff99fc on k8snetworkplumbingwg:master.

@adrianchiris
Copy link
Contributor Author

adrianchiris commented Jun 16, 2024

An alternative solution is to actually clean up multus cni config file on exit if cleanup-config-on-exit flag is set in thin_entrypoint[1]

note: thick plugin today cleans up multus config file on exit[2] so im thinking we should support the same in thin mode as well.

[1]

if opt.CleanupConfigOnExit && opt.MultusConfFile == "auto" {

[2]
os.Remove(multusConfigFile)

@s1061123
Copy link
Member

We use distroless for less maintainance for CVE in container image because distroless image, gcr.io/distroless/base-debian11:latest, has less components than distro image, debian:stable-slim. So we (or I, at least), expects to change thick pluign container image to distroless in the future. Not to change thin plugin to distro image.

Currently our release container image is just for simple use cases and it does not care about whole lifecycle for all deployment scenarios (on all platforms). If you want to do more, we expect that you should have original container image for that.

So I suppose we should not change thin pluign base images.

@adrianchiris
Copy link
Contributor Author

if we are ok with the approach in #1299 then this one can be closed.

@adrianchiris
Copy link
Contributor Author

closing this one in fafor of #1299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants